Hi @G3mha! Thanks for opening a PR into RAWeb!

Username lookups are already case-insensitive. Please try hitting prod with your favorite API testing tool:

GET https://retroachievements.org/API/API_GetUserSummary.php?y=YOUR_WEB_API_KEY&u=NICOPLAYSTHINGS

{
	"User": "NicoPlaysThings",
	"MemberSince": "2021-02-06 23:36:29",
	"LastActivity": {
		"ID": 0,
		"timestamp": null,
		"lastupdate": null,
		"activitytype": null,
		"User": "NICOPLAYSTHINGS",
		"data": null,
		"data2": null
	},
	"RichPresenceMsg": "Mario is exploring Overworld ⭐[65\/70]",
	"LastGameID": 17700,
	"ContribCount": 0,
	"ContribYield": 0,
	"TotalPoints": 16631,
	"TotalSoftcorePoints": 0,
	"TotalTruePoints": 25619,
	"Permissions": 1,
	"Untracked": 0,
	"ID": 165298,
	"UserWallActive": 1,
	"Motto": "",
	"Rank": 3477,
	"RecentlyPlayedCount": 0,
	"RecentlyPlayed": [],
	"UserPic": "\/UserPic\/NICOPLAYSTHINGS.png",
	"TotalRanked": 78279,
	"Awarded": {},
	"RecentAchievements": {},
	"Status": "Offline"
}

Notice in this response from prod:

"User": "NicoPlaysThings",

So it appears to me the case-corrected value is already available - it just needs to be used instead of the query param where appropriate.

@wescopeland Thank you for the clarification, however I'm unable to find the canonicalUser if in API the code executes:

// Get canonical username from db
$canonicalUser = User::byDisplayName($user)->value('User');
if (!$canonicalUser) {
    return response()->json([
        'ID' => null,
        'User' => $user,
    ], 404);
}

If the method searches up for a canonical username using a non-canonical username to do so in the DB.

public function scopeByDisplayName(Builder $query, string $username): Builder
    {
        return $query->where('User', $username);
    }

When I do so hitting the test with username=NICOPLAYSTHINGS, I get an error on the lookup:

php artisan test tests/Feature/Api/V1/UserSummaryTest.php --filter testUsernameCaseConsistency
array:2 [
  "ID" => null
  "User" => "NICOPLAYSTHINGS"
] // tests/Feature/Api/V1/UserSummaryTest.php:501

I agree with you that in production the API returns the canonicalUsername, not case-sensitive. However, by querying the DB, it is indeed case-sensitive.

Could you please help me up understand how to get a better picture of the problem?

Hi @G3mha!

I agree with you that in production the API returns the canonicalUsername, not case-sensitive. However, by querying the DB, it is indeed case-sensitive.

Try running:

SHOW TABLE STATUS WHERE Name = 'UserAccounts';

This will output a bunch of metadata about the table, including the collation name (which determines case-sensitivity of queries). You should see a Collation value suffixed with "_ci". "ci" stands for case-insensitive.

SELECT * FROM UserAccounts WHERE User = "WCopeland"; -- 1 row returned
SELECT * FROM UserAccounts WHERE User = "wcopeland"; -- 1 row returned
SELECT * FROM UserAccounts WHERE User = "WCOPELAND"; -- 1 row returned

If queries to this table were case-sensitive, then ?u= NICOPLAYSTHINGS on the API endpoint wouldn't work.

In API_GetUserSummary, there's a call to getUserInfo(), which is making the query:

$user = User::firstWhere('User', $username);

If you place dd($user->User) immediately after this line and run the code, you will see the case-corrected username output regardless of the input case of the $username variable.

API_GetUserSummary is a very high-traffic endpoint. Unfortunately, we probably cannot afford to add another DB query in it 😞

However, I believe you should be able to solve this with values already available in the endpoint code 🙂

The problem is LastActivity.User and UserPic in the response are incorrect. Focus on why the casings for those values are incorrect and how to get the case-corrected value, and I think you will be on the right track 🙂

That is perfect @wescopeland, however the test for it is not passing.

It appears to me that MySQL is non case sensitive, but SQLite (on the tests) is case-sensitive. Could you point out to me how to change the testing from SQLite to MySQL? Or make my SQL non case sensitive?

I've inserted two dumps on API_GetUserSummary.php, in this section (so you can better understand the output):

dump($user);
$retVal = getUserPageInfo($user, $recentGamesPlayed, $recentAchievementsEarned);
dump($retVal);

And the output of the test was this:

php artisan test tests/Feature/Api/V1/UserSummaryTest.php --filter testUsernameCaseConsistency
"NICOPLAYSTHINGS" // public/API/API_GetUserSummary.php:106
[] // public/API/API_GetUserSummary.php:108

   FAIL  Tests\Feature\Api\V1\UserSummaryTest
  ⨯ username case consistency                                                                                                          0.51s  
  ──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────  
   FAILED  Tests\Feature\Api\V1\UserSummaryTest > username case consistency                                                                   
  Expected response status code [200] but received 404.
Failed asserting that 404 is identical to 200.

  at tests/Feature/Api/V1/UserSummaryTest.php:498
    494▕ 
    495▕         foreach ($variations as $input) {
    496▕             $response = $this->get($this->apiUrl('GetUserSummary', ['u' => $input]));
    497▕ 
  ➜ 498▕             $response->assertOk()
    499▕                     ->assertJsonPath('User', 'NicoPlaysThings')
    500▕                     ->assertJsonPath('LastActivity.User', 'NicoPlaysThings')
    501▕                     ->assertJsonPath('UserPic', '/UserPic/NicoPlaysThings.png');
    502▕         }


  Tests:    1 failed (1 assertions)
  Duration: 0.53s

Regrettably, it appears SQLite will not let us get test coverage of this fix due to the collation differences. At the moment we're not equipped to run tests in MySQL/MariaDB.

I don't like to do this, but for now let's omit writing a new test case for this change. I'll verify manually.

Given that, we'll still need to remove this from the endpoint code:

$canonicalUser = User::byDisplayName($user)->value('User');

@wescopeland I understand it. Certainly, not the best practice, but as Jim Gordon in the Dark Knight would say "I don't get [...] points for being an idealist, I have to do the best I can with what I have."

I just kept on this PR the essential changes, therefore, just the API_GetUserSummary.php. Could you review it for me?